Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recursive structs supported #335

Closed
wants to merge 18 commits into from

Conversation

RyougiNevermore
Copy link

just use processing cache, and no performance loss.

codec_record.go Outdated Show resolved Hide resolved
@RyougiNevermore
Copy link
Author

I'm not sure about the results of golangci-lint. any help?

nrwiersma and others added 6 commits December 21, 2023 19:42
* origin/main:
  feat: move generic decoding to codec level (hamba#336)
  fix: linter issues

# Conflicts:
#	config.go
TestEncoder_RecordMapNilValue passed
@RyougiNevermore
Copy link
Author

uhhh, if support disable cache mode, maybe use some one like ctx, and it will cost more performance, from 1400 ns/op up to 1700 ns/op in.
and there is a bug about big.rat in generic decoder..., so can use *big.rat insteadof big.rat?

Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase and clean up. Seems a lot of stuff is left over or unused.

}

type processingGroup struct {
group *singleflight.Group
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed for the local option.

type encoderOfTypeHandler func(cfg *frozenConfig, p *processing, schema Schema, typ reflect2.Type) ValEncoder

type processing struct {
key []byte
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont get what this is used for?

@@ -0,0 +1,46 @@
// Package mmhash export runtime.memhash
package mmhash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be unused.

@nrwiersma
Copy link
Member

Fixed by #413

@nrwiersma nrwiersma closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants